testutil: support empty string args in fakecommand #244
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
flotter
force-pushed
the
test-util-fakecommand
branch
from
4a3e3e5
to
261a969
Compare
flotter
changed the title
flotter
force-pushed
the
test-util-fakecommand
branch
from
261a969
to
12acc94
Compare
flotter
changed the title
FakeCommand allows unit tests to check which command-line arguments
were passed, by recording a log of calls with their arguments.
The following examples cannot be tested with FakeCommand:
1. shutdown -r "+0" ""
2. echo ""
3. configure --program-prefix ""
The empty string argument confuses the command parser in testutil.
The issue is that both the argument (\000) and command delimiter
(\000\000) uses null characters, which makes the command split
operation split on the first occurrence of two null chars. This
is an invalid assumption for an empty string argument, which will
be presented as two argument delimiters, without anything in
between, appearing like a command delimiter.
To add support for this:
- Change the command delimiter from \000\000 => \000\f\n\r (magic sequence)
- Update the Calls() function to split the lines correctly.
Testutil can now be used for testing the following command, including the
case where msg is an empty string for the wall message:
shutdown [OPTIONS...] [TIME] [WALL...]
cmd := exec.Command("shutdown", "-r", fmt.Sprintf("+%d", mins), msg)
flotter
force-pushed
the
test-util-fakecommand
branch
from
12acc94
to
641012b
Compare
|
This was an enhancement I did for #243 which is now closed. |
atesburak
approved these changes
Jun 22, 2023
anpep
approved these changes
Jun 26, 2023
|
@jnsgruk I did the work to enhance the testutil library to test the changes in the now closed PR. When I closed the PR, I decided that this may be a useful enhancement to keep, so I broke this out into its own PR. |
jnsgruk
approved these changes
Aug 1, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
testutil: support empty string args in fakecommand
FakeCommand allows unit tests to check which command-line arguments
were passed, by recording a log of calls with their arguments.
The following examples cannot be tested with FakeCommand:
shutdown -r "+0" ""
echo ""
configure --program-prefix ""
The empty string argument confuses the command parser in testutil.
The issue is that both the argument (\000) and command delimiter
(\000\000) uses null characters, which makes the command split
operation split on the first occurrence of two null chars. This
is an invalid assumption for an empty string argument, which will
be presented as two argument delimiters, without anything in
between, appearing like a command delimiter.
To add support for this:
Change the command delimiter from \000\000 => \000\f\n\r (magic sequence)
Update the Calls() function to split the lines correctly.
Testutil can now be used for testing the following command, including the
case where msg is an empty string for the wall message:
shutdown [OPTIONS...] [TIME] [WALL...]
cmd := exec.Command("shutdown", "-r", fmt.Sprintf("+%d", mins), msg)